Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/doc: Add Developing the Test Driver #216660

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

roberth
Copy link
Member

@roberth roberth commented Feb 16, 2023

Description of changes

Describe how to work on the NixOS Test Driver.

Main reason to write this was the test process.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation labels Feb 16, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Feb 16, 2023

As we currently have some flaky tests, newly failing tests are expected, but should be reviewed to make sure that
- The number of failures did not increase significantly.
- All failures that do occur can reasonably be assumed to fail for a different reason than the changes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document itself is good.

At the same time, what do you think about rigorously dropping everything what's flaky? Flaky tests are a nuisance to everyone who invests good work to get changes through and have the side-effect of wearing down motivation to painstaking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we don't mistake flaky Hydra for flaky tests, this would absolutely be a win.

We could perhaps move them to a flaky attrset, so that they can be fixed and moved back when they have a record of being reliable again.

Reasons to prefer dropping over moving:

  • less is more
  • satisfying

Reasons to prefer moving:

  • the state of the test remains visible, so that someone can pick it up and debug it
  • some tests need to evolve to a non-flaky state, as race conditions are discovered
  • no loss of coverage
  • Hermit

Hermit emulates the scheduler deterministically. This should let us find debuggable counterexamples, I think. Might be too new/experimental, and might not support KVM. We'll want to run it only on the flaky tests, because it is slower by an order of magnitude, but NixOS isn't poor so that's not a reason to avoid it.

So yeah, I think moving those tests would be great!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having different sets of which only one is really mandatory for contributors to satisfy sounds like a great improvement, yes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #216828 for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, @JulienMalka wanted to explore Hermit usage in NixOS tests, I don't know if he has something to share, but just mentioning him as it can be interesting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I think it'd be very interesting to deep a little further into Hermit, see what improvements in test reproducibility we can get and at what cost. I'll certainly work on that subject soon, available to chat more about it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What parts of the nixos test driver stack would be touched by using hermit? My gut feel says that this would be "just" configuring the guest vm's nixos config with kernel patch + config properly, is that about right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally Hermit supports KVM, so that we only have to wrap the top level invocation in nixos/lib/testing/run.nix. Otherwise, we have a qemu invocation in nixos/modules/virtualisation/qemu-vm.nix, which is included (a bit indirectly) from nixos/lib/testing/nodes.nix. Configuring the kernel works the way it normally does in NixOS, I think.

@roberth roberth force-pushed the nixos-doc-develop-test-driver branch from fb07a3c to 9964891 Compare February 17, 2023 17:18
@roberth
Copy link
Member Author

roberth commented Feb 17, 2023

@ofborg eval

@roberth roberth merged commit 505feab into NixOS:master Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants